Chat: tell a banned user what happened and when it lifts - #3479
Conversation
The provider detected the ban but kept only the message, discarding bannedUntil and reason, so the UI could not tell a 48-hour timeout from a 3-year ban. It showed a one-shot toast: 'Unusual activity detected. Please try again after some time.' A ban is a state, not an event, so once that toast was dismissed every later send failed with no standing explanation. Preserves the payload and replaces the toast with a persistent banner above the composer, carrying the same copy and duration bands as the web client so the two clients cannot describe one moderation action differently. The banner ticks rather than freezing at first render, and clears itself both at expiry and on a successful send, which is how an early moderator unban is picked up. The tick is a bounded interval: a delay derived from bannedUntil overflows the 32-bit setTimeout limit and would fire immediately for a multi-year ban. The old toast stays as the fallback for a ban with no usable expiry, so an older server still says something rather than nothing.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c70cc2345f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // A send that lands proves the ban is gone, which is how an early moderator unban | ||
| // clears the banner instead of it lingering until the original expiry. | ||
| setBanInfo(null); |
There was a problem hiding this comment.
When a banned user successfully edits an existing message, _handleSend takes the updateMattermostMessage PATCH branch and then clears banInfo here. A successful edit does not prove that the separate create-post restriction has been lifted, so the standing notice disappears even though subsequent new messages may still be rejected; move this clear into the sendMattermostMessage branch.
Useful? React with 👍 / 👎.
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe chat flow now preserves structured ban metadata, formats localized duration-aware notices, and displays a persistent banner until the ban expires. The container clears the banner after successful sends or expiration. ChangesChat ban notices
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Mattermost
participant ChatThreadContainer
participant chatBanNotice
participant ChatBanBanner
Mattermost-->>ChatThreadContainer: ban error with bannedUntil and reason
ChatThreadContainer->>chatBanNotice: getChatBanInfo(error)
chatBanNotice-->>ChatThreadContainer: ChatBanInfo
ChatThreadContainer->>ChatBanBanner: render(info)
ChatBanBanner->>chatBanNotice: formatChatBanNotice(info, now)
chatBanNotice-->>ChatBanBanner: localized notice
ChatBanBanner-->>ChatThreadContainer: onExpire()
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/screens/chats/children/ChatBanBanner.tsx`:
- Around line 25-26: Move the onExpireRef.current assignment out of render and
into a useEffect associated with the ChatBanBanner component, updating it only
after the render commits. Preserve the existing ref usage while preventing
discarded concurrent renders from changing the callback used by the committed
interval.
- Line 46: Update the formatChatBanNotice call in ChatBanBanner to pass
intl.formatMessage directly, removing the any cast and preserving the existing
formatter behavior.
In `@src/screens/chats/container/chatThreadContainer.tsx`:
- Around line 1326-1328: Update onNewMessage’s pendingMatch || fallbackMatch
branch to clear banInfo after WebSocket send confirmation, ensuring the cleanup
runs even when the subsequent HTTP request fails. Preserve the existing banner
behavior for messages without a confirmed pending or fallback match.
In `@src/screens/chats/utils/chatBanNotice.ts`:
- Around line 25-35: Update getChatBanInfo validation to reject all non-finite
bannedUntil values by using Number.isFinite(bannedUntil) alongside the existing
presence and expiry checks. Preserve the current null return and valid
future-expiry handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ee21c3ed-ef88-49c3-813b-b69cd8eed3b3
📒 Files selected for processing (6)
src/config/locales/en-US.jsonsrc/providers/chat/mattermost.tssrc/screens/chats/children/ChatBanBanner.tsxsrc/screens/chats/container/chatThreadContainer.tsxsrc/screens/chats/utils/chatBanNotice.test.tssrc/screens/chats/utils/chatBanNotice.ts
The clear sat after the edit/create branches, so it ran for both. Editing an existing message is not ban-gated server-side (only the create-post route checks isUserChatBanned), so a banned user could edit a message, succeed, and dismiss the notice while still blocked from posting anything new. Moved into the create branch, keyed on the request the ban actually gates.
Websocket confirmation of our own send is independent proof the create landed, and it can arrive when the HTTP response never does. Without clearing there the banner sat until its original expiry despite the ban being lifted. Both match arms are create-only, so an edit still cannot dismiss it. getChatBanInfo used Number.isNaN, which Infinity passes, and Infinity is also greater than now, so both guards let it through: the banner would render an endless duration and never fire onExpire. Switched to Number.isFinite, with a test covering Infinity, -Infinity, the string form and a plain non-number. Moved the onExpire ref assignment into an effect so a discarded render cannot mutate the ref the committed interval reads, and dropped the unnecessary any cast on intl.formatMessage (IntlShape's signature already satisfies the injected formatter type; typecheck confirms).
1 participant
Closes #3470. Mobile counterpart to ecency/vision-web#1390 (merged).
Before
sendMattermostMessagedetected the ban but built an error carrying only the message, droppingbannedUntilandreason. The UI showed a one-shot toast:No duration, so a 48-hour timeout and a 3-year ban read identically. And because it was a toast, once dismissed every later send failed with no standing explanation.
After
A persistent banner above the composer:
Changes
providers/chat/mattermost.tspreservesbannedUntilandreasonon the thrown errorscreens/chats/utils/chatBanNotice.tsbuilds the copy, mirroring the web formatter with the same reasons and the same duration bands, so the two clients cannot describe one moderation action differently.formatMessageis injected rather than imported, keeping it a pure testable unitscreens/chats/children/ChatBanBanner.tsxrenders it above the composer, next to the action that is blocked, following the existingDmWarningBannerpatternchats.ban-*keys added to the Crowdin sourceDetails worth knowing
The banner ticks. It re-renders on a bounded interval instead of freezing at first render, and clears both at expiry and on a successful send, which is how an early moderator unban gets picked up without a reload.
The tick is deliberately a fixed interval, not a delay derived from
bannedUntil.setTimeouttakes a 32-bit signed delay, so a 3-year ban resolves to ~1ms and would clear its own notice instantly, hiding it from exactly the users who are most banned. That bit the web PR; there is a test for it here.Duration bands never produce a count of 1, so nothing reads "in about 1 hours" and these stay plain keys rather than needing plural variants.
The old toast remains as a fallback for a ban with no usable expiry, so an older server still says something rather than nothing.
Unknown reasons degrade to generic copy: live bans predate the reason prop, and a newer moderation service may add reasons this build has not seen.
Checks
Full suite green (756 passed, 51 suites), 14 new tests.
typecheck ok: 0 errors (baseline 0). Lint on the touched files is unchanged fromdevelopment(12 warnings, 0 errors, all pre-existing).The locale addition is a byte-preserving textual insert. A
json.dumpround-trip rewrote existing\uXXXXescapes across the file, so the diff here is 10 added lines rather than a whole-file reformat that would have churned the Crowdin source.Summary by CodeRabbit
New Features
Bug Fixes
Tests